Add multiplayer counter sample by jhuleatt · Pull Request #1273 · firebase/functions-samples

jhuleatt · 2026-04-17T13:57:48Z

No description provided.

Co-authored-by: kevmoo <kevmoo@google.com> Co-authored-by: Rody Davis <rody.davis.jr@gmail.com>

gemini-code-assist

Code Review

This pull request introduces a multi-counter application using Flutter, Firebase, and Dart, organized within a workspace containing app, server, and shared packages. Key features include Google authentication, real-time Firestore synchronization for user and global statistics, and a Dart-based Cloud Function for incrementing counters. Feedback focuses on optimizing the server-side counter logic to use atomic increments instead of $O(N)$ aggregations, removing redundant authentication token calls in the client, and improving security by restricting CORS policies. Additionally, suggestions were made to improve navigation performance by moving initialization logic out of the router and ensuring the UI is properly notified of successful operations.

gemini-code-assist · 2026-04-17T13:59:54Z

+  Future<void> increment(String userId) async {
+    try {
+      await _increment(userId);
+      await _updateGlobalCount();
+    } catch (e, stack) {
+      print('Error incrementing counter for user: $userId');
+      print(e);
+      print(stack);
+      rethrow;
+    }
+  }


The current implementation updates the global count by performing a full aggregation over the users collection on every increment. This is an $O(N)$ operation that will not scale as the user base grows.

Instead, you should update the global counters within the same transaction as the user's increment using FieldValue.increment. This ensures atomicity and provides $O(1)$ performance. Once implemented, the helper methods _increment and _updateGlobalCount can be removed.

Future<void> increment(String userId) async { try { await _firestore.runTransaction<void>((transaction) async { final userRef = _firestore.collection(usersCollection).doc(userId); final globalRef = _firestore.collection(globalCollection).doc(varsDocument); final userSnapshot = await transaction.get(userRef); if (!userSnapshot.exists) { transaction.set(userRef, {countField: 1}); transaction.update(globalRef, { totalCountField: const FieldValue.increment(1), totalUsersField: const FieldValue.increment(1), }); } else { transaction.update(userRef, {countField: const FieldValue.increment(1)}); transaction.update(globalRef, { totalCountField: const FieldValue.increment(1), }); } }); } catch (e, stack) { print('Error incrementing counter for user: $userId'); print(e); print(stack); rethrow; } }

I see what she's saying - but we lose the ability to guarantee we stay in sync!

gemini-code-assist · 2026-04-17T13:59:54Z

+    final user = FirebaseAuth.instance.currentUser;
+    if (user == null) {
+      _responseController.add(
+        IncrementResponse.failure('User is not authenticated.'),
+      );
+      return;
+    }
+
+    final idToken = await user.getIdToken();
+    if (idToken == null) {
+      _responseController.add(
+        IncrementResponse.failure('User is not authenticated.'),
+      );
+      return;
+    }


The call to user.getIdToken() is redundant. The Firebase Functions HttpsCallable automatically handles authentication tokens when a user is signed in. Additionally, the retrieved idToken is not used in the subsequent call.

Suggested change

final user = FirebaseAuth.instance.currentUser;

if (user == null) {

_responseController.add(

IncrementResponse.failure('User is not authenticated.'),

);

return;

}

final idToken = await user.getIdToken();

if (idToken == null) {

_responseController.add(

IncrementResponse.failure('User is not authenticated.'),

);

return;

}

final user = FirebaseAuth.instance.currentUser;

if (user == null) {

_responseController.add(

IncrementResponse.failure('User is not authenticated.'),

);

return;

}

not a bad idea, although very unlikely, given the auth setup

gemini-code-assist · 2026-04-17T13:59:54Z

+    }
+
+    try {
+      await incrementHttpsCallable.call<void>();


The _callIncrement method should notify the _responseController when the operation succeeds so the UI can provide feedback to the user (e.g., clearing a loading state or showing a success message).

Suggested change

await incrementHttpsCallable.call<void>();

await incrementHttpsCallable.call<void>();

_responseController.add(IncrementResponse.success());

gemini-code-assist · 2026-04-17T13:59:54Z

+  }
+
+  void _handleIncrementResult(_) {
+    // TODO: handle the result


This TODO should be addressed. You should handle the result of the increment operation, for example by logging success or updating internal state if needed.

not sure what to do here. This method no longer does...anything. but it's the target of the stream.

@rodydavis ?

gemini-code-assist · 2026-04-17T13:59:54Z

+
+      options: const CallableOptions(
+        // TODO: should be explicit here about the supported hosts
+        cors: OptionLiteral(['*']),


Using a wildcard * for CORS is insecure as it allows any domain to call this function. It is recommended to specify the actual allowed origins (e.g., your app's hosting URL).

gemini-code-assist · 2026-04-17T13:59:54Z

+  ],
+  redirect: (context, state) async {
+    // Ensure we capture the redirect result before checking the auth state.
+    await FirebaseAuth.instance.getRedirectResult();


getRedirectResult() should typically be called once during app initialization (e.g., in main or a dedicated initialization block), rather than inside the router's redirect function. Calling it here causes it to execute on every navigation, which is inefficient.

Maybe? I haven't tried it...

I think I put it here when fighting the other auth bits... Need to investigate.

* docs: add README for Dart multi_counter sample Adds a README to the Dart/multi_counter directory highlighting the sample was presented at Cloud Next 2026. Includes a link to the session. Also updates the root README.md to reference this sample. Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…endencies (#1275) * ci: setup flutter to build pure dart or flutter workspace projects Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> * ci: fix unused import in firebase_options.dart causing dart analyze failure Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> * ci: fix unused import in firebase_options.dart causing dart analyze failure Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> * ci: fix unused import in firebase_options.dart causing dart analyze failure Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> * ci: fix dart format exit code on lib/firebase_options.dart Co-authored-by: jhuleatt <3759507+jhuleatt@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

kevmoo

A whole pile of comments!

kevmoo · 2026-04-19T00:21:48Z

+  Stream<IncrementResponse> get incrementResponseStream =>
+      _responseController.stream;
+
+  // TODO: consider creating shared constants for collection and field names.


I think we ca ditch this comment

kevmoo · 2026-04-19T00:21:54Z

+      _responseController.stream;
+
+  // TODO: consider creating shared constants for collection and field names.
+  // ...and putting them in the shared package.


kevmoo · 2026-04-19T00:22:10Z

+    }
+  }
+
+  // TODO: consider making this a nullable-property and disabling


ditch this comment, too

kevmoo · 2026-04-19T00:22:33Z

+    final user = FirebaseAuth.instance.currentUser;
+    if (user == null) {
+      _responseController.add(
+        IncrementResponse.failure('User is not authenticated.'),
+      );
+      return;
+    }
+
+    final idToken = await user.getIdToken();
+    if (idToken == null) {
+      _responseController.add(
+        IncrementResponse.failure('User is not authenticated.'),
+      );
+      return;
+    }


not a bad idea, although very unlikely, given the auth setup

kevmoo · 2026-04-19T00:23:07Z

+  }
+
+  void _handleIncrementResult(_) {
+    // TODO: handle the result


not sure what to do here. This method no longer does...anything. but it's the target of the stream.

@rodydavis ?

kevmoo · 2026-04-19T00:24:05Z

+    );
+  } else {
+    return FirebaseFunctions.instance.httpsCallableFromUrl(
+      'https://increment-138342796561.us-central1.run.app',


should we just put example.com here or something?

or do we put a throw in here with a TODO to populate it?

kevmoo · 2026-04-19T00:24:24Z

+const double doubleSpaceSize = spaceSize * 2;
+
+// TODO: Update this to the final example URI
+const githubDisplayUrl = 'github.com/kevmoo/next26_demo';


Should likely ditch this and the bits that depend on it for the sample!

kevmoo · 2026-04-19T00:25:00Z

+  ],
+  redirect: (context, state) async {
+    // Ensure we capture the redirect result before checking the auth state.
+    await FirebaseAuth.instance.getRedirectResult();


Maybe? I haven't tried it...

I think I put it here when fighting the other auth bits... Need to investigate.

kevmoo · 2026-04-19T00:25:48Z

+      name: incrementCallable,
+
+      options: const CallableOptions(
+        // TODO: should be explicit here about the supported hosts


The TODO is REAL – hrm... we could update it for the user to update when they know where they are deploying?

kevmoo · 2026-04-19T00:26:30Z

+  Future<void> increment(String userId) async {
+    try {
+      await _increment(userId);
+      await _updateGlobalCount();
+    } catch (e, stack) {
+      print('Error incrementing counter for user: $userId');
+      print(e);
+      print(stack);
+      rethrow;
+    }
+  }


I see what she's saying - but we lose the ability to guarantee we stay in sync!

Import multi-counter sample

ac1ba48

Co-authored-by: kevmoo <kevmoo@google.com> Co-authored-by: Rody Davis <rody.davis.jr@gmail.com>

jhuleatt requested review from kevmoo and rodydavis April 17, 2026 13:57

gemini-code-assist Bot reviewed Apr 17, 2026

View reviewed changes

jhuleatt and others added 4 commits April 17, 2026 10:32

remove Firebase config object

6e74cea

update lockfile

745370d

kevmoo reviewed Apr 19, 2026

View reviewed changes

	await incrementHttpsCallable.call<void>();
	await incrementHttpsCallable.call<void>();
	_responseController.add(IncrementResponse.success());

Conversation

jhuleatt commented Apr 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevmoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants